Skip to content

fix: push_repo_memory should not run when agent job is skipped or failed#24363

Merged
pelikhan merged 1 commit intomainfrom
copilot/fix-agent-job-run-conditions
Apr 3, 2026
Merged

fix: push_repo_memory should not run when agent job is skipped or failed#24363
pelikhan merged 1 commit intomainfrom
copilot/fix-agent-job-run-conditions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 3, 2026

push_repo_memory was unconditionally guarded by always(), so it ran even when the agent job failed or was skipped — pointlessly consuming a runner and risking writing stale/incomplete memory.

Changes

  • pkg/workflow/repo_memory.go: Added needs.agent.result == 'success' to the push_repo_memory job condition, matching the identical guard already present on update_cache_memory
  • Lock files: Recompiled all 183 workflows

Before / After (with threat detection):

# Before
if: always() && (needs.detection.result == 'success' || needs.detection.result == 'skipped')

# After
if: always() && (needs.detection.result == 'success' || needs.detection.result == 'skipped') && needs.agent.result == 'success'

Without threat detection:

# Before
if: always()

# After
if: always() && needs.agent.result == 'success'

@pelikhan pelikhan marked this pull request as ready for review April 3, 2026 19:56
Copilot AI review requested due to automatic review settings April 3, 2026 19:56
@pelikhan pelikhan merged commit 84049b3 into main Apr 3, 2026
51 checks passed
@pelikhan pelikhan deleted the copilot/fix-agent-job-run-conditions branch April 3, 2026 19:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the generated GitHub Actions workflow logic so push_repo_memory only runs when the agent job succeeds, preventing wasted runner usage and avoiding writing stale/incomplete repo memory.

Changes:

  • Tightened push_repo_memory job-level if: to additionally require needs.agent.result == 'success' (and still gate on detection pass/skip when threat detection is enabled).
  • Regenerated compiled workflow lock files to reflect the updated condition across workflows.
Show a summary per file
File Description
pkg/workflow/repo_memory.go Updates push_repo_memory job condition to require agent success (and detection pass/skip when enabled).
.github/workflows/workflow-health-manager.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/weekly-blog-post-writer.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/technical-doc-writer.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/security-compliance.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/pr-triage-agent.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/metrics-collector.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/glossary-maintainer.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/firewall-escape.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/discussion-task-miner.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/developer-docs-consolidator.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/delight.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/deep-report.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-testify-uber-super-expert.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-news.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-copilot-token-report.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-community-attribution.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-code-metrics.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/daily-cli-performance.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/copilot-session-insights.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/copilot-pr-prompt-analysis.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/copilot-pr-nlp-analysis.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/copilot-cli-deep-research.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/copilot-agent-analysis.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/code-scanning-fixer.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/audit-workflows.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.
.github/workflows/agent-performance-analyzer.lock.yml Regenerated lock to include needs.agent.result == 'success' in push_repo_memory condition.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 27/27 changed files
  • Comments generated: 2

Comment on lines +738 to +739
// failed or was skipped). Using always() so the job still runs even when upstream jobs
// are skipped (e.g. detection is skipped when agent produces no outputs).
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function-level doc comment for buildPushRepoMemoryJob earlier in this file still states the job runs even if the agent job fails, but the updated jobCondition now requires needs.agent.result == 'success'. Please update that doc comment to match the new behavior so future readers aren’t misled.

Suggested change
// failed or was skipped). Using always() so the job still runs even when upstream jobs
// are skipped (e.g. detection is skipped when agent produces no outputs).
// failed or was skipped). Using always() so the condition is still evaluated consistently
// when other upstream jobs are skipped (e.g. detection is skipped when agent produces no
// outputs), but the job still requires the agent job result to be "success".

Copilot uses AI. Check for mistakes.
Comment on lines +740 to 752
agentSucceeded := BuildEquals(
BuildPropertyAccess(fmt.Sprintf("needs.%s.result", constants.AgentJobName)),
BuildStringLiteral("success"),
)
jobNeeds := []string{string(constants.AgentJobName)}
var jobCondition string
if threatDetectionEnabled {
// When threat detection is enabled, run only if detection succeeded (no threats found)
// or was skipped (agent produced no outputs or patch — nothing to detect against).
jobCondition = RenderCondition(BuildAnd(BuildFunctionCall("always"), buildDetectionPassedCondition()))
// When threat detection is enabled, also require detection passed (succeeded or skipped).
jobCondition = RenderCondition(BuildAnd(BuildAnd(BuildFunctionCall("always"), buildDetectionPassedCondition()), agentSucceeded))
jobNeeds = append(jobNeeds, string(constants.DetectionJobName))
} else {
jobCondition = RenderCondition(BuildAnd(BuildFunctionCall("always"), agentSucceeded))
}
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change updates the generated job-level if: for push_repo_memory, but there doesn’t appear to be a test asserting the new condition (agent success required, plus detection passed/skipped when enabled). Adding a unit/integration test that validates pushRepoMemoryJob.If contains the expected clauses would help prevent regressions (similar to the existing update_cache_memory condition coverage).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants